SECURITY: Prevent tokens in jsonp mode

Add checks to token-returning functions to prevent returning tokens in
jsonp mode. This affects action=tokens, action=login,
action=createaccount, and action=query&list=deletedrevs.

Also, remove the "gettoken" parameter to action=block and
action=unblock, which has been deprecated since 1.20.

Bug: 49090
Change-Id: Ibeaa5c72d8084585092b15935a3f5709104bf7f7

diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index 859d6c9..f89cbe2 100644 (file)
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -338,6 +338,9 @@ production.
   version of the title.
 * (bug 52538) action=edit will now use empty text instead of the contents
   of section 0 when passed prependtext or appendtext with section=new.
+* Support for the 'gettoken' parameter to action=block and action=unblock,
+  deprecated since 1.20, has been removed.
+* (bug 49090) Token-getting functions will fail when using jsonp callbacks.
 
 === Languages updated in 1.22===
 

diff --git a/includes/api/ApiBlock.php b/includes/api/ApiBlock.php
index ab0a7e9..975153a 100644 (file)
--- a/includes/api/ApiBlock.php
+++ b/includes/api/ApiBlock.php
@@ -42,12 +42,6 @@ class ApiBlock extends ApiBase {
                $user = $this->getUser();
                $params = $this->extractRequestParams();
 
-               if ( $params['gettoken'] ) {
-                       $res['blocktoken'] = $user->getEditToken();
-                       $this->getResult()->addValue( null, $this->getModuleName(), $res );
-                       return;
-               }
-
                if ( !$user->isAllowed( 'block' ) ) {
                        $this->dieUsageMsg( 'cantblock' );
                }
@@ -156,10 +150,6 @@ class ApiBlock extends ApiBase {
                                ApiBase::PARAM_REQUIRED => true
                        ),
                        'token' => null,
-                       'gettoken' => array(
-                               ApiBase::PARAM_DFLT => false,
-                               ApiBase::PARAM_DEPRECATED => true,
-                       ),
                        'expiry' => 'never',
                        'reason' => '',
                        'anononly' => false,
@@ -177,7 +167,6 @@ class ApiBlock extends ApiBase {
                return array(
                        'user' => 'Username, IP address or IP range you want to block',
                        'token' => 'A block token previously obtained through prop=info',
-                       'gettoken' => 'If set, a block token will be returned, and no other action will be taken',
                        'expiry' => 'Relative expiry time, e.g. \'5 months\' or \'2 weeks\'. If set to \'infinite\', \'indefinite\' or \'never\', the block will never expire.',
                        'reason' => 'Reason for block',
                        'anononly' => 'Block anonymous users only (i.e. disable anonymous edits for this IP)',
@@ -194,10 +183,6 @@ class ApiBlock extends ApiBase {
        public function getResultProperties() {
                return array(
                        '' => array(
-                               'blocktoken' => array(
-                                       ApiBase::PROP_TYPE => 'string',
-                                       ApiBase::PROP_NULLABLE => true
-                               ),
                                'user' => array(
                                        ApiBase::PROP_TYPE => 'string',
                                        ApiBase::PROP_NULLABLE => true

diff --git a/includes/api/ApiCreateAccount.php b/includes/api/ApiCreateAccount.php
index 201634d..0e752c5 100644 (file)
--- a/includes/api/ApiCreateAccount.php
+++ b/includes/api/ApiCreateAccount.php
@@ -29,6 +29,10 @@
  */
 class ApiCreateAccount extends ApiBase {
        public function execute() {
+               // If we're in JSON callback mode, no tokens can be obtained
+               if ( !is_null( $this->getMain()->getRequest()->getVal( 'callback' ) ) ) {
+                       $this->dieUsage( 'Cannot create account when using a callback', 'aborted' );
+               }
 
                // $loginForm->addNewaccountInternal will throw exceptions
                // if wiki is read only (already handled by api), user is blocked or does not have rights.

diff --git a/includes/api/ApiLogin.php b/includes/api/ApiLogin.php
index b936d3b..b51d441 100644 (file)
--- a/includes/api/ApiLogin.php
+++ b/includes/api/ApiLogin.php
@@ -46,6 +46,15 @@ class ApiLogin extends ApiBase {
         * is reached. The expiry is $this->mLoginThrottle.
         */
        public function execute() {
+               // If we're in JSON callback mode, no tokens can be obtained
+               if ( !is_null( $this->getMain()->getRequest()->getVal( 'callback' ) ) ) {
+                       $this->getResult()->addValue( null, 'login', array(
+                               'result' => 'Aborted',
+                               'reason' => 'Cannot log in when using a callback',
+                       ) );
+                       return;
+               }
+
                $params = $this->extractRequestParams();
 
                $result = array();

diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index 4dd1713..c10d85c 100644 (file)
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -714,15 +714,9 @@ class ApiMain extends ApiBase {
                }
                $moduleParams = $module->extractRequestParams();
 
-               // Die if token required, but not provided (unless there is a gettoken parameter)
-               if ( isset( $moduleParams['gettoken'] ) ) {
-                       $gettoken = $moduleParams['gettoken'];
-               } else {
-                       $gettoken = false;
-               }
-
+               // Die if token required, but not provided
                $salt = $module->getTokenSalt();
-               if ( $salt !== false && !$gettoken ) {
+               if ( $salt !== false ) {
                        if ( !isset( $moduleParams['token'] ) ) {
                                $this->dieUsageMsg( array( 'missingparam', 'token' ) );
                        } else {

diff --git a/includes/api/ApiQueryDeletedrevs.php b/includes/api/ApiQueryDeletedrevs.php
index 690d0e6..8273313 100644 (file)
--- a/includes/api/ApiQueryDeletedrevs.php
+++ b/includes/api/ApiQueryDeletedrevs.php
@@ -57,6 +57,11 @@ class ApiQueryDeletedrevs extends ApiQueryBase {
                $fld_content = isset( $prop['content'] );
                $fld_token = isset( $prop['token'] );
 
+               // If we're in JSON callback mode, no tokens can be obtained
+               if ( !is_null( $this->getMain()->getRequest()->getVal( 'callback' ) ) ) {
+                       $fld_token = false;
+               }
+
                $result = $this->getResult();
                $pageSet = $this->getPageSet();
                $titles = $pageSet->getTitles();

diff --git a/includes/api/ApiTokens.php b/includes/api/ApiTokens.php
index 7080f54..d220a5e 100644 (file)
--- a/includes/api/ApiTokens.php
+++ b/includes/api/ApiTokens.php
@@ -48,6 +48,11 @@ class ApiTokens extends ApiBase {
        }
 
        private function getTokenTypes() {
+               // If we're in JSON callback mode, no tokens can be obtained
+               if ( !is_null( $this->getMain()->getRequest()->getVal( 'callback' ) ) ) {
+                       return array();
+               }
+
                static $types = null;
                if ( $types ) {
                        return $types;

diff --git a/includes/api/ApiUnblock.php b/includes/api/ApiUnblock.php
index 55e7331..6a739a2 100644 (file)
--- a/includes/api/ApiUnblock.php
+++ b/includes/api/ApiUnblock.php
@@ -39,12 +39,6 @@ class ApiUnblock extends ApiBase {
                $user = $this->getUser();
                $params = $this->extractRequestParams();
 
-               if ( $params['gettoken'] ) {
-                       $res['unblocktoken'] = $user->getEditToken();
-                       $this->getResult()->addValue( null, $this->getModuleName(), $res );
-                       return;
-               }
-
                if ( is_null( $params['id'] ) && is_null( $params['user'] ) ) {
                        $this->dieUsageMsg( 'unblock-notarget' );
                }
@@ -96,10 +90,6 @@ class ApiUnblock extends ApiBase {
                        ),
                        'user' => null,
                        'token' => null,
-                       'gettoken' => array(
-                               ApiBase::PARAM_DFLT => false,
-                               ApiBase::PARAM_DEPRECATED => true,
-                       ),
                        'reason' => '',
                );
        }
@@ -110,7 +100,6 @@ class ApiUnblock extends ApiBase {
                        'id' => "ID of the block you want to unblock (obtained through list=blocks). Cannot be used together with {$p}user",
                        'user' => "Username, IP address or IP range you want to unblock. Cannot be used together with {$p}id",
                        'token' => "An unblock token previously obtained through prop=info",
-                       'gettoken' => 'If set, an unblock token will be returned, and no other action will be taken',
                        'reason' => 'Reason for unblock',
                );
        }
@@ -118,10 +107,6 @@ class ApiUnblock extends ApiBase {
        public function getResultProperties() {
                return array(
                        '' => array(
-                               'unblocktoken' => array(
-                                       ApiBase::PROP_TYPE => 'string',
-                                       ApiBase::PROP_NULLABLE => true
-                               ),
                                'id' => array(
                                        ApiBase::PROP_TYPE => 'integer',
                                        ApiBase::PROP_NULLABLE => true

diff --git a/tests/phpunit/includes/api/ApiBlockTest.php b/tests/phpunit/includes/api/ApiBlockTest.php
index 3c18968..d0eb18a 100644 (file)
--- a/tests/phpunit/includes/api/ApiBlockTest.php
+++ b/tests/phpunit/includes/api/ApiBlockTest.php
@@ -62,22 +62,6 @@ class ApiBlockTest extends ApiTestCase {
                $this->assertEquals( 'infinity', $block->mExpiry );
        }
 
-       /**
-        * @dataProvider provideBlockUnblockAction
-        */
-       function testGetTokenUsingABlockingAction( $action ) {
-               $data = $this->doApiRequest(
-                       array(
-                               'action' => $action,
-                               'user' => 'UTApiBlockee',
-                               'gettoken' => '' ),
-                       null,
-                       false,
-                       self::$users['sysop']->user
-               );
-               $this->assertEquals( 34, strlen( $data[0][$action]["{$action}token"] ) );
-       }
-
        /**
         * Attempting to block without a token should give a UsageException with
         * error message: